Skip to content

[rig-tasks] Add 10 rig samples — 2026-08-07 - #365

Merged
pelikhan merged 1 commit into
mainfrom
rig-tasks/2026-08-07-ba8735e48fe8e8da
Aug 8, 2026
Merged

[rig-tasks] Add 10 rig samples — 2026-08-07#365
pelikhan merged 1 commit into
mainfrom
rig-tasks/2026-08-07-ba8735e48fe8e8da

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Added 10 new rig sample files to skills/rig/samples/.

# File Description Kind Typecheck
1 371-git-file-ownership-mapper.md Git file ownership mapper via git log agent ✅ pass
2 372-workflow-input-validator.md GitHub Actions workflow_dispatch input validator agent ✅ pass
3 373-ts-dead-export-finder.md TypeScript dead export finder agent ✅ pass
4 374-package-scripts-documenter.md package.json scripts documenter → SCRIPTS.md agent ✅ pass
5 375-git-diff-stats-summarizer.md Git diff --numstat summarizer with s.enum change types agent ✅ pass
6 376-dotenv-template-generator.md .env.template generator from process.env refs agent ✅ pass
7 377-toml-config-extractor.md TOML config section/key extractor (new) agent ✅ pass
8 378-csv-column-stats.md CSV column statistics reporter (new) agent ✅ pass
9 379-git-worktree-analyzer.md Git worktree listing analyzer (new) agent ✅ pass
10 380-ts-narrowing-detector.md TypeScript narrowing pattern detector (new) agent ✅ pass

Typecheck failures

One failure was encountered and fixed before writing the sample file:

Task 2 (372-workflow-input-validator): The local type annotation { default?: string } conflicted with exactOptionalPropertyTypes: true when assigning string | undefined. Fixed by widening to { default?: string | undefined }.

Tasks run

  • (reused) Git file ownership mapper
  • (reused) GitHub Actions workflow input validator
  • (reused) TypeScript dead export finder
  • (reused) package.json scripts documenter
  • (reused) Git diff stats summarizer
  • (reused) Dotenv template generator
  • (new) TOML config file key extractor
  • (new) CSV column statistics reporter
  • (new) Git worktree listing analyzer
  • (new) TypeScript type narrowing pattern detector

Generated by Daily Rig Task Generator · sonnet46 107.2 AIC · ⌖ 9.46 AIC · ⊞ 6.8K ·

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review August 8, 2026 18:45
@pelikhan
pelikhan merged commit 2282354 into main Aug 8, 2026
1 check passed
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /diagnosing-bugs and /codebase-design — requesting changes on correctness issues found across several samples.

📋 Key Themes & Highlights

Key Themes

  • Shell injection (371): execSync with a template-string filePath — switch to spawnSync with an argument array.
  • Algorithmic false negatives (373): String.includes for symbol lookup produces false negatives; use word-boundary regex.
  • Schema/tool mismatch (374): command field in output schema has no corresponding tool return — the LLM must hallucinate it.
  • Unreliable CWD heuristic (379): process.cwd() to detect the main worktree is wrong outside the repo root; use the porcelain structure instead.
  • Regex coverage gaps (372, 380): YAML indent assumptions miss 2-space workflows; typeof pattern misses !==.
  • Optional field handling (377, 378): s shadowed in reduce; explicit undefined returned for s.optional fields.

Positive Highlights

  • ✅ All 10 samples typecheck cleanly, including the exactOptionalPropertyTypes fix in 372.
  • ✅ Consistent use of repair() addon on samples with structured output — good default.
  • s.enum used correctly for discriminated string fields (e.g. changeType, category).
  • p.readInput / p.readOptional used appropriately in 376, 377, 378 — idiomatic rig patterns.
  • steering() chosen for open-ended analysis agents (371, 379); repair() for structured extraction — correct pairing.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 50.2 AIC · ⌖ 4.96 AIC · ⊞ 6.3K
Comment /matt to run again

description: "Get git commit history for a file and return ownership info.",
parameters: { filePath: s.path },
handler: ({ filePath }: { filePath: string }) => {
try {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] Shell injection risk: filePath is interpolated directly into the shell command string without sanitization — paths containing shell metacharacters (spaces, $(), backticks) will be interpreted by the shell.

💡 Fix: use spawnSync with argument array

Switch from a template-string execSync call to spawnSync with a proper argument array, so the path is never parsed by the shell:

import { spawnSync } from "node:child_process";
const result = spawnSync("git", ["log", "--format=%ae", "--", filePath], { encoding: "utf-8" });
const output = result.stdout.trim();

Agent-generated tool inputs are a realistic injection vector since the LLM controls the filePath argument.

if (other === filePath) continue;
try {
const otherContent = await readFile(other, "utf-8");
if (otherContent.includes(sym)) { found = true; break; }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] False negative: otherContent.includes(sym) matches any substring, so a symbol like log will be considered "used" if any file contains logError, blogPost, etc. This produces incorrect dead-export results.

💡 Fix: use a word-boundary regex match
const usagePattern = new RegExp(`\\b${sym}\\b`);
if (usagePattern.test(otherContent)) { found = true; break; }

Word-boundary matching is the minimum needed to avoid false negatives on short symbol names.

command: s.string,
})
),
documentedCount: s.int,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] Schema/tool mismatch: the output schema declares scripts: s.record(s.object({ purpose, category, command })), but inferScriptPurpose only returns { purpose, category } — it never returns command. The LLM must hallucinate command from context, which is unreliable.

💡 Fix: either return `command` from the tool or remove it from the output schema

Option A — return command from the tool:

return { purpose, category, command } as const;

Option B — drop command from the output schema since the agent instructions can describe the scripts table without it.

The output schema should be the single source of truth; every field the LLM must populate should have a clear data source in the instructions or tool returns.

parameters: { path: s.string, branch: s.string, commit: s.string, bare: s.boolean, detached: s.boolean },
handler: ({ path, branch, commit, bare, detached }: { path: string; branch: string; commit: string; bare: boolean; detached: boolean }) => {
const type: "main" | "linked" | "bare" = bare ? "bare" : path === process.cwd() ? "main" : "linked";
const status: "clean" | "dirty" | "detached" = detached ? "detached" : "clean";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] process.cwd() inside a tool handler always returns the CLI's working directory, not the git repository root. On any invocation where the repo root differs from CWD — or in future non-stdio transports — this heuristic will misclassify the main worktree as "linked".

💡 Fix: use git to identify the main worktree

The porcelain output already marks the main worktree (the first block has no worktree prefix before the path); use that structural signal instead of CWD comparison:

// In the agent instructions: track whether the current block is the first one
// and pass an `isFirst` boolean to classifyWorktree.
const type: "main" | "linked" | "bare" = bare ? "bare" : isFirst ? "main" : "linked";

Alternatively, git worktree list marks the main worktree as the one with no gitdir field — use that from the parsed output.

try {
const content = await readFile(filePath, "utf-8");
const typeofCount = (content.match(/\btypeof\s+\w+\s*[=!]==/g) ?? []).length;
const instanceofCount = (content.match(/\binstanceof\b/g) ?? []).length;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] The typeof regex /\btypeof\s+\w+\s*[=!]==/g only matches === and !==, so it misses the common patterns typeof x == 'string' (double equals) and typeof x === 'string' with a string literal on the right (the == char class [=!] is followed by = but typeof x === "number" ends in three = signs — confirmed match). More critically, it won't match typeof x !== 'string' because !== ends in = not = after !=.

💡 Fix: broaden the pattern to match both operand styles
// Match: typeof x === ..., typeof x !== ..., typeof x == ..., typeof x != ...
const typeofCount = (content.match(/\btypeof\s+\w+\s*[!=]==?/g) ?? []).length;

Also consider counting all typeof occurrences rather than only narrowing guard forms, since typeof x in non-comparison contexts (e.g. logging) won't be captured.

}
const totalSections = Object.keys(sections).filter((k) => k !== "__default__").length;
const totalKeys = Object.values(sections).reduce((sum, s) => sum + Object.keys(s).length, 0);
const result: Record<string, Record<string, string>> = {};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] Variable name s in the reduce callback shadows the imported s schema helper from "rig". TypeScript will not catch this because s inside the callback refers to the callback parameter, but it creates a confusing naming collision in a file that prominently uses s.* for schemas.

💡 Fix: rename the reduce accumulator
const totalKeys = Object.values(sections).reduce((sum, kvs) => sum + Object.keys(kvs).length, 0);

Using sum for the accumulator avoids the shadow entirely.

parameters: { filePath: s.path },
handler: async ({ filePath }: { filePath: string }) => {
const content = await readFile(filePath, "utf-8");
const inputsMatch = content.match(/workflow_dispatch:\s*\n(?:\s+.*\n)*?\s+inputs:([\s\S]*?)(?=\n\w|\n\s{0,2}\w|$)/);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] The inputsMatch regex is fragile: it requires workflow_dispatch: to be followed immediately by inputs: within a narrow look-ahead, and uses a fixed indentation depth of 4–8 spaces for inputPattern. Real-world workflow files often use 2-space indentation (GitHub's default), which will be silently missed, returning { inputs: {}, inputCount: 0 } for valid workflow files.

💡 Suggestion: use a YAML parser or loosen the indentation constraint

The simplest fix is to widen the indent range in inputPattern:

const inputPattern = /^\s{2,}(\w+):\s*\n((?:\s{3,}.+\n?)*)/gm;

A more robust fix would be to use a lightweight YAML parser (e.g. js-yaml) instead of regex, which is explicitly what the sample's docstring implies the agent does. Since this is a sample, a comment noting the limitation is also acceptable.

return { columnName, type, uniqueCount, min, max, mean };
}
return { columnName, type, uniqueCount, min: undefined, max: undefined, mean: undefined };
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] analyzeColumn returns { min: undefined, max: undefined, mean: undefined } for non-numeric columns, but the output schema declares these as s.optional(s.number). In s.object, s.optional means the key may be absent — not that it may be undefined. Returning explicit undefined values may cause schema validation to fail depending on how the serializer handles them.

💡 Fix: omit the keys entirely for non-numeric columns
// Instead of: return { columnName, type, uniqueCount, min: undefined, ... }
return { columnName, type, uniqueCount };

Omitting optional keys is the correct pattern with s.optional; explicit undefined is only safe if the runtime strips it before validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant